Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sap_general_preconfigure: Revert to ignoring DNS failures #785

Merged

Conversation

berndfinger
Copy link
Member

Solves issue #784.

Copy link
Member

@sean-freeman sean-freeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

@berndfinger berndfinger removed the request for review from rhmk June 27, 2024 12:35
@berndfinger
Copy link
Member Author

@rhmk Is not available this week, so I removed him from the list of reviewers.
@rob0d Please have a look at this PR and comment.

@rob0d
Copy link
Contributor

rob0d commented Jun 29, 2024

@berndfinger yes, it would make it safer and more flexible to make the strict check optional and use something like sap_general_preconfigure_dns_check_strict: false as a default.
We can possibly also enhance the PTR check to search through all returned records in a hope that one will match as mentioned in #784 as it will possibly help with the Azure behaviour that @sean-freeman mentioned. I'm not sure if it's worth the effort though.
I can have a look at this, but I am away until 15th July.

@berndfinger
Copy link
Member Author

I'm going to merge this PR now to get back the previous default behavior while also implementing an option to enforce strict DNS checking. We can continue discussing further options in the issue and see if we want or can improve the DNS checking later.

@berndfinger berndfinger merged commit 36a5525 into sap-linuxlab:dev Jul 1, 2024
3 of 4 checks passed
@berndfinger berndfinger deleted the issue-784-revert-to-dns-warning branch July 1, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants